Change classes and constructors to new/primary syntax. #2233
Conversation
Package publishing
Documentation at https://github.com/dart-lang/ecosystem/wiki/Publishing-automation. |
PR HealthChangelog Entry ❗
Changes to files need to be accounted for in their respective changelogs. This check can be disabled by tagging the PR with
Coverage
|
| File | Coverage |
|---|---|
| pkgs/package_config/bin/package_config_of.dart | 💔 Not covered |
| pkgs/package_config/lib/src/errors.dart | 💔 0 % ⬇️ 100 % |
| pkgs/package_config/lib/src/package_config.dart | 💔 0 % ⬇️ 100 % |
This check for test coverage is informational (issues shown here will not fail the PR).
This check can be disabled by tagging the PR with skip-coverage-check.
License Headers ✔️
// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
| Files |
|---|
| no missing headers |
All source files should start with a license header.
Unrelated files missing license headers
| Files |
|---|
| pkgs/bazel_worker/benchmark/benchmark.dart |
| pkgs/benchmark_harness/integration_test/perf_benchmark_test.dart |
| pkgs/boolean_selector/example/example.dart |
| pkgs/clock/lib/clock.dart |
| pkgs/clock/lib/src/clock.dart |
| pkgs/clock/lib/src/default.dart |
| pkgs/clock/lib/src/stopwatch.dart |
| pkgs/clock/lib/src/utils.dart |
| pkgs/clock/test/clock_test.dart |
| pkgs/clock/test/default_test.dart |
| pkgs/clock/test/stopwatch_test.dart |
| pkgs/clock/test/utils.dart |
| pkgs/coverage/lib/src/coverage_options.dart |
| pkgs/html/example/main.dart |
| pkgs/html/lib/dom.dart |
| pkgs/html/lib/dom_parsing.dart |
| pkgs/html/lib/html_escape.dart |
| pkgs/html/lib/parser.dart |
| pkgs/html/lib/src/constants.dart |
| pkgs/html/lib/src/encoding_parser.dart |
| pkgs/html/lib/src/html_input_stream.dart |
| pkgs/html/lib/src/list_proxy.dart |
| pkgs/html/lib/src/query_selector.dart |
| pkgs/html/lib/src/token.dart |
| pkgs/html/lib/src/tokenizer.dart |
| pkgs/html/lib/src/treebuilder.dart |
| pkgs/html/lib/src/utils.dart |
| pkgs/html/test/dom_test.dart |
| pkgs/html/test/parser_feature_test.dart |
| pkgs/html/test/parser_test.dart |
| pkgs/html/test/query_selector_test.dart |
| pkgs/html/test/selectors/level1_baseline_test.dart |
| pkgs/html/test/selectors/level1_lib.dart |
| pkgs/html/test/selectors/selectors.dart |
| pkgs/html/test/support.dart |
| pkgs/html/test/tokenizer_test.dart |
| pkgs/html/test/trie_test.dart |
| pkgs/html/tool/generate_trie.dart |
| pkgs/pubspec_parse/test/git_uri_test.dart |
| pkgs/stack_trace/example/example.dart |
| pkgs/watcher/test/custom_watcher_factory_test.dart |
| pkgs/yaml_edit/example/example.dart |
This check can be disabled by tagging the PR with skip-license-check.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the codebase to use new Dart 3 syntax, including primary constructors and interface classes. The changes are mostly correct and align with modern Dart practices. However, I've found a few critical issues in pkgs/package_config/lib/src/errors.dart where implements clauses were dropped during refactoring, breaking the type hierarchy. I've also identified several places where the new syntax for factories and constructors is used incorrectly. I've provided suggestions to fix these issues.
| @sealed | ||
| class SimpleInvalidLanguageVersion extends SimpleLanguageVersion | ||
| class SimpleInvalidLanguageVersion(String source) | ||
| extends SimpleLanguageVersion(-1, -1, source) |
There was a problem hiding this comment.
Without super-class arguments, this will be an in-body
new(String source): super(-1, -1, source);Just wanted to show what it would look like with super-class arguments. (Because I probably wouldn't use a primary constructor without it.)
| ) : super.value(); | ||
|
|
||
| PackageConfigArgumentError.from(ArgumentError error) | ||
| ) : super.value(); |
There was a problem hiding this comment.
I initially made this a primary constructor (using super-class arguments), but since all construction needs to go through the primary constructor, and this constructor restricts the types to non-nullable to require a name and message, the from() constructor could not redirect to it.
(That could be an argument for preferring an in-body declaring constructor, if you want other initializing constructors on the same class, which cannot be made to forward to the primary constructor.)
(DO NOT LAND!)
Example of using new constructor syntax on a code-base.
I used
SuperClass(args)/SuperClass.name(args)as well. In every case where I did that, I would not have used a primary constructor otherwise.